Conversation
Contributor
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #1672 +/- ##
==========================================
+ Coverage 98.02% 98.84% +0.81%
==========================================
Files 179 181 +2
Lines 16422 16745 +323
Branches 3776 3815 +39
==========================================
+ Hits 16098 16551 +453
+ Misses 244 105 -139
- Partials 80 89 +9
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
chfast
force-pushed
the
amsterdam/8037
branch
from
September 7, 2026 13:17
78114ae to
6dc7d64
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The mocked host mishandles a valid zero state-gas result, and important meter, system-call, and block-accounting coverage is missing.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Implements EIP-8037’s Amsterdam two-dimensional execution/state-gas model across EVMC, VM execution, transaction processing, and block accounting.
Changes:
- Adds state-gas reservoirs, spilling, refilling, and frame rollback.
- Charges state creation for accounts, storage, and deployed code.
- Adds dimensional validation, receipts, block accounting, and tests.
File summaries
| File | Description |
|---|---|
test/utils/test_state.hpp |
Extends transition helper API. |
test/utils/test_state.cpp |
Forwards the state-gas budget. |
test/utils/statetest_runner.cpp |
Supplies the statetest state-gas budget. |
test/utils/error_matching.cpp |
Adds gas-overflow exception mapping. |
test/utils/block_transition.cpp |
Implements dimensional block accounting; needs nonzero state-gas aggregation coverage. |
test/unittests/state_tx_test.cpp |
Updates validation calls. |
test/unittests/state_transition.hpp |
Adds expected state-gas fields. |
test/unittests/state_transition.cpp |
Verifies receipt state gas. |
test/unittests/state_transition_eip8037_test.cpp |
Adds EIP-8037 transition tests. |
test/unittests/state_transition_create_test.cpp |
Updates deployment gas limits. |
test/unittests/CMakeLists.txt |
Registers new tests. |
test/state/transaction.hpp |
Adds gas dimensions to transaction properties and receipts. |
test/state/system_contracts.cpp |
Adds the system-call state reservoir; boundary coverage is missing. |
test/state/state.hpp |
Extends the transaction validation API. |
test/state/state.cpp |
Implements validation, gas splitting, and settlement. |
test/state/host.cpp |
Implements account and code state charging. |
test/state/account.hpp |
Adds an account-liveness helper. |
lib/evmone/state_gas.hpp |
Defines reservoir accounting; focused boundary tests are needed. |
lib/evmone/instructions.hpp |
Adds child merging and SELFDESTRUCT charging. |
lib/evmone/instructions_storage.cpp |
Implements SSTORE state gas. |
lib/evmone/instructions_calls.cpp |
Implements CALL/CREATE state gas. |
lib/evmone/execution_state.hpp |
Tracks and returns frame state gas. |
lib/evmone/constants.hpp |
Defines EIP-8037 costs. |
evmc/include/evmc/mocked_host.hpp |
Propagates mocked state gas, but incorrectly overloads zero as default behavior. |
evmc/include/evmc/evmc.hpp |
Exposes result fields in C++. |
evmc/include/evmc/evmc.h |
Extends ABI structures and bumps the ABI. |
Review details
Suppressed comments (3)
lib/evmone/state_gas.hpp:31
- The new consensus-critical meter has no direct unit coverage: the added transition tests do not exercise atomic charge failure, a mixed reservoir/spill charge, LIFO refill, or
repay_spill()after a child refund. Please add focused boundary tests for these operations so pool-conservation regressions are caught independently of transaction plumbing.
[[nodiscard]] bool charge(int64_t& gas_left, int64_t cost) noexcept
test/state/system_contracts.cpp:80
- The referenced
system_contract_reaches_gas_limittest is not present in the repository, leaving the new consensus-visible 16-SSTORE reservoir unverified. Add an Amsterdam system-call regression that exercises this exact reservoir boundary (and the behavior immediately beyond it), rather than documenting a test that does not exist.
/// Cap on the number of SSTOREs a system contract may fund out of its state-gas budget. The value
/// is observable: `system_contract_reaches_gas_limit` sizes a contract to exactly
/// `30M + SYSTEM_MAX_SSTORES_PER_CALL × STORAGE_SET_STATE_GAS` (EIP-8037).
constexpr int64_t SYSTEM_MAX_SSTORES_PER_CALL = 16;
test/utils/block_transition.cpp:123
- No block test exercises this new two-dimensional aggregation with nonzero state gas; the existing EIP-7778 tooling test uses only a storage clear, whose state component is zero. Add a multi-transaction Amsterdam case with differing regular/state components so it distinguishes
max(sum_regular, sum_state)from summing per-transaction maxima and verifies both remaining-dimension checks.
// The block's 2D gas formula (EIP-7778).
const auto block_gas_used = std::max(sum_regular_gas, sum_state_gas);
- Files reviewed: 26/26 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+413
to
+416
| // A zero state_gas_left means "the callee consumed the caller's whole reservoir". | ||
| // The mock runs no code, so echo the reservoir it was handed unless a test set one. | ||
| if (result.state_gas_left == 0) | ||
| result.state_gas_left = msg.state_gas; |
chfast
force-pushed
the
amsterdam/8037
branch
10 times, most recently
from
September 16, 2026 08:40
7e852b3 to
fa6e8f1
Compare
chfast
force-pushed
the
amsterdam/8037
branch
2 times, most recently
from
September 16, 2026 14:34
cb58950 to
5e39d06
Compare
Introduce the Amsterdam two-dimensional gas model: state-creation costs move out of regular gas into a separate state-gas dimension, priced at COST_PER_STATE_BYTE (1530) per byte of new state. - evmc: add `state_gas` to the message and `state_gas_left`/`state_gas_spilled` to the result, threading a per-frame state-gas reservoir through the VM. - StateGas (state_gas.hpp): a (reservoir-left, spilled) pair. Charges draw from the reservoir first and spill into regular gas_left; refunds refill in LIFO order; a frame's net use derives as `initial - left + spilled`. Frames roll their state gas back on revert/halt (make_execution_result). - Charges at state-creation sites: new account by CREATE/CREATE2 (at the deployment-address access), by value-CALL — including the depth-0 value-transfer charge the EIP-2780 decomposition later builds on — and by SELFDESTRUCT to a new beneficiary (NEW_ACCOUNT = 120 bytes); SSTORE 0->non-zero slot allocation (64 bytes, with the 0->Y->0 LIFO refill; the regular set cost drops to its 2900 component); code deposit per byte. Failed creations refund the charge. Opcode CREATE and the create transaction keep the legacy 32000 execution cost here: EIP-8037 defers its execution component to EIP-8038's CREATE_ACCESS, and EIP-8038 states that the flat GAS_CREATE is what CREATE_ACCESS replaces, so the reprice lands with it. - Transaction processing: execution gas splits into a regular budget (capped by TX_MAX_GAS_LIMIT - intrinsic) and the state-gas reservoir. Amsterdam lifts the Osaka per-tx gas cap; validation instead caps the regular intrinsic and applies the per-dimension block-inclusion rules against the new block state-gas budget. - Block accounting: per-tx receipts carry regular/state components; block gas_used = max(sum_regular, sum_state) (EIP-7778 2D formula). - System calls get a separate 16-SSTORE state-gas reservoir so the state dimension cannot OOG them. The intrinsic cost otherwise keeps the pre-Amsterdam formula; the EIP-2780 resource decomposition lands separately. The EIP-7702 per-authorization state charges (AUTH_BASE and the authority's NEW_ACCOUNT) are not part of this commit: they are only expressible through the top-frame charging model that the EIP-2780 intrinsic decomposition introduces, so they land with it. The intrinsic keeps the pre-Amsterdam formula here. Includes the state-gas unit tests and the GAS_ALLOWANCE_EXCEEDED / BlockException.GAS_USED_OVERFLOW acceptance, which exists because the per-dimension inclusion checks keep an over-block-gas transaction a transaction-level rule.
chfast
force-pushed
the
amsterdam/8037
branch
from
September 16, 2026 19:21
16a5f09 to
10938ce
Compare
Host::call patched the state-gas fields of every failed result because Host::create returned bare failures with zeroed ones. Build them with the baseline instead, which leaves the patch asserting the contract evmc.h already states. The REVERT spill return it also carried was dead: a frame's spill is settled by make_execution_result before the result reaches here.
It returns the state-gas reservoir alongside the execution gas limit since EIP-8037, but both doc comments still described a single limit.
The out-of-gas result was constructed for every transaction and overwritten on the normal path. Select it with the host call instead, and drop the charge_succeeded branch around the settlement: a failed charge leaves both counters untouched, so the failure arm is already a no-op for it.
chfast
force-pushed
the
amsterdam/8037
branch
from
September 17, 2026 09:13
a67dfd5 to
27db065
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implement EIP-8037: "State Creation Gas Cost Increase"
Introduce the Amsterdam two-dimensional gas model: state-creation costs move
out of regular gas into a separate state-gas dimension, priced at
COST_PER_STATE_BYTE (1530) per byte of new state.
state_gasto the message andstate_gas_left/state_gas_spilledto the result, threading a per-frame state-gas reservoir through the VM.
from the reservoir first and spill into regular gas_left; refunds refill in
LIFO order; a frame's net use derives as
initial - left + spilled. Framesroll their state gas back on revert/halt (make_execution_result).
deployment-address access), by value-CALL — including the depth-0
value-transfer charge the EIP-2780 decomposition later builds on — and by
SELFDESTRUCT to a new beneficiary (NEW_ACCOUNT = 120 bytes); SSTORE
0->non-zero slot allocation (64 bytes, with the 0->Y->0 LIFO refill; the
regular set cost drops to its 2900 component); code deposit per byte.
Failed creations refund the charge. Opcode CREATE and the create
transaction keep the legacy 32000 execution cost here: EIP-8037 defers its
execution component to EIP-8038's CREATE_ACCESS, and EIP-8038 states that
the flat GAS_CREATE is what CREATE_ACCESS replaces, so the reprice lands
with it.
by TX_MAX_GAS_LIMIT - intrinsic) and the state-gas reservoir. Amsterdam
lifts the Osaka per-tx gas cap; validation instead caps the regular
intrinsic and applies the per-dimension block-inclusion rules against the
new block state-gas budget.
block gas_used = max(sum_regular, sum_state) (EIP-7778 2D formula).
dimension cannot OOG them.
The intrinsic cost otherwise keeps the pre-Amsterdam formula; the EIP-2780
resource decomposition lands separately.
The EIP-7702 per-authorization state charges (AUTH_BASE and the authority's
NEW_ACCOUNT) are not part of this commit: they are only expressible through
the top-frame charging model that the EIP-2780 intrinsic decomposition
introduces, so they land with it. The intrinsic keeps the pre-Amsterdam
formula here.
Includes the state-gas unit tests and the GAS_ALLOWANCE_EXCEEDED /
BlockException.GAS_USED_OVERFLOW acceptance, which exists because the
per-dimension inclusion checks keep an over-block-gas transaction a
transaction-level rule.